contracts: feature: introduce realm levels #1721
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Failed to generate code suggestions for PR |
![mentatbot[bot]](https://avatars.githubusercontent.com/in/837875?s=60&v=4){kind=small}
There was a problem hiding this comment.
The implementation of realm levels looks solid and well-integrated with existing systems. The new functionality includes a level field in the Realm struct, a RealmLevelConfig for storing level-up requirements, and an upgrade_level function in the realm systems contract. The building creation process now checks the realm's level before allowing placement, which is a good way to enforce level restrictions.
The tests are comprehensive and cover various scenarios, including successful upgrades, insufficient resources, and attempts to upgrade beyond the max level. The max level is currently hardcoded to 3, which might be worth reconsidering for future flexibility.
Overall, the changes seem well-thought-out and follow established patterns in the codebase. Consider adding more documentation for the realm level mechanics and possibly emitting events for level upgrades to improve observability.
Thanks for using MentatBot. Give comments a 👍 or 👎 to help me improve!
contracts/src/models/realm.cairo
Outdated
| impl RealmCustomImpl of RealmCustomTrait { | ||
| fn max_level(self: Realm) -> u8 { |
There was a problem hiding this comment.
Consider making the max_level configurable rather than hardcoded. This would allow for easier adjustments in the future if needed.
| @@ -178,5 +180,42 @@ mod realm_systems { | |||
|
|
|||
There was a problem hiding this comment.
Consider emitting an event when a realm is upgraded. This would be useful for tracking realm progress and could be used by the frontend or other systems.
| let directions_count = directions.len(); | ||
| assert!(directions_count > 0, "building cant be made at the center"); | ||
| assert!(directions_count <= realm.max_level().into() + 1, "building outside of max bound"); | ||
| assert!(directions_count <= realm.level.into() + 1, "building outside of what realm level allows"); |
There was a problem hiding this comment.
The error message 'building outside of what realm level allows' could be more informative. Consider including the current realm level and the required level in the message.
| assert!(directions_count <= realm.level.into() + 1, "building outside of what realm level allows"); | |
| assert!(directions_count <= realm.level.into() + 1, | |
| &format!("building outside of what realm level allows: current level is {}, required level is {}", realm.level, directions_count - 1)); |
contracts/src/models/realm.cairo
Outdated
| impl RealmCustomImpl of RealmCustomTrait { | ||
| fn max_level(self: Realm) -> u8 { | ||
| 3 |
There was a problem hiding this comment.
change this to config
|
In config i have added some contants which can be used to init the config, can you update to use them |
contracts for #1719